Skip to content

Merging SaX-based parsing support to increase read performance#12

Open
Wohlstand wants to merge 63 commits into
masterfrom
wip-mdx
Open

Merging SaX-based parsing support to increase read performance#12
Wohlstand wants to merge 63 commits into
masterfrom
wip-mdx

Conversation

@Wohlstand

Copy link
Copy Markdown
Member

This is a large work done by @ds-sloth to increase the performance and reduce the memory usage. I think it's ready to merge, so, I creating this as an easy-to-review thing. So, any notices I will address to @ds-sloth who is an author of the most of these changes.

@Wohlstand Wohlstand self-assigned this May 31, 2026
Comment thread src/mdx/mdx_level_file.cpp Outdated
MDX_FIELD("BG", background_id);
MDX_FIELD("AS", autoscrol);
MDX_FIELD_NONNEG("AST", autoscroll_style);
MDX_UNIQUE_FIELD("ASP", MDX_LevelEvent_load_autoscroll_path, nullptr); // FIXME

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ds-sloth, what the trouble with this field? Actually it's easy to produce it using the next way:

  • Open SMBX-38A level file with the autoscroll path built
  • Save as LVLX
  • Data is ready!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem was that saving wasn't previously implemented. So we could load but not save it. This should be fixed now.

Comment on lines +380 to +390
static bool s_save_junk_line(const void* _FileData, PGESTRING& obj, pge_size_t index)
{
const WorldData& FileData = *reinterpret_cast<const WorldData*>(_FileData);

if(index >= FileData.unsupported_38a_lines.size())
return false;

obj = FileData.unsupported_38a_lines[index];

return true;
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These lines actually never been saved by PGE-X generators and never been supported by PGE-X parsers. These lines basically preserved by PGE-FL for SMBX-38A files and returned back into the file when saving as SMBX-38A format. They never saved when saving to PGE-X format, and they gets just lost.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, you'd prefer that I remove support for them from the MDX engine?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've checked and these are actually here for 38A support, not MDX support. Because the 38A loader is also SAX-based now.

Comment thread src/smbx38a/file_rw_lvl_38a.cpp
ds-sloth and others added 24 commits June 8, 2026 15:27
To be more obvious what role of this section.
Otherwise, the warning for C files will appear
Since std::runtime_error and other std::exceptions do support it too.
Since some of public heads do use without full directory binding.
Making a large diff hunk in honor for code style!

Such diff hunk is absolutely no problem (you always can filter whitespace changes in the diff!), the problem that code style gets messed up!
…ems"

This reverts commit e229c7c.

Needed to revert due to previous versions of TheXTech requiring the ID field
@Wohlstand

Copy link
Copy Markdown
Member Author

I rebased the branch so now it's fully compatible to the "master".

@ds-sloth

Copy link
Copy Markdown
Contributor

I addressed the comments and also fixed a minor issue: you partially restored the original way that saved layers were implemented (in the header), but they got changed later and are now implemented in a different way (as their own section). You can see the commits linked above for reference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants